-
Notifications
You must be signed in to change notification settings - Fork 215
Fix Zend_Pdf_Element regression by added $value prop #490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Zend_Pdf_Element regression by added $value prop #490
Conversation
|
Well.. seems, that UTs haven't been updated yet to cover this eventuality. Any chance to add explicit test on this, so we wont run into this in next iteration? |
|
We should add |
|
Was reverted by rector. I've addeda PR to your branch. |
Update .rector.php
|
Specific UT is still preferred here, thanks. |
|
Please point me to the current test for this method. |
I think any type from tests/Zend/Pdf/Element/ could be extended to check the behaviour, so it will fail, when changed one more time |
|
Let's merge and add UT in a separate PR. And i don't know how to test it. |
| */ | ||
| abstract class Zend_Pdf_Element | ||
| { | ||
| public $value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it worked setting to protected $value; to generate a PDF with a PNG image. Otherwise the PDF just stays blank without an image. I guess the protected value can't get accessed directly, thus the magical getter kicks in to retrieve value from the stream object.
include 'vendor/autoload.php';
$filename = '/var/www/testfile.png';
# @see https://github.com/Shardj/zf1-future/pull/453#issuecomment-2416109612
# "public $value" in Zend_Pdf_Element causes image not to render in PDF.
# Switching to "protected $value" worked for me.
$image = Zend_Pdf_Image::imageWithPath($filename);
$pdf = new Zend_Pdf();
$page = new Zend_Pdf_Page(Zend_Pdf_Page::SIZE_A4);
$page->drawImage($image, 50, 500, 400, 800);
$pdf->pages[] = $page;
header('Content-Type: application/pdf');
header('Content-Disposition: inline; filename="output.pdf"');
echo $pdf->render();PS: But if specific rector rule is still needed, just leave the code like you suggested. Openmage with "release-1.24.1" is not working for me at the moment. So I'm happy if this Pull Request is merged soon. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For OM feel free to start discussing about alternatives.
dompdf, ..., twig ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my use case I'm using a third party module for invoice creation.
By @sreichel's Phpstan error fixes in #475 / v1.24.3, the same regression was reintroduced again, which I have fixed last Oct in #453.
Please read #453 thread for more details.